No problem with the square brackets, but should this be noted as an issue with the main parsing algorithm, @nk-coding ? I must admit I don't fully understand what the issue is there, perhaps @amirrza777 you can explain in a bit more detail?

Happy to go into more detail. The grammar looks like this (csvImportRules.ts):

CsvImportContentRule: "{" many(or(add("imports", CsvClassImportRule), NEWLINE)) "}"
CsvClassImportRule: class, "from", file, optional("{", many(mapping), "}") <- original, ambiguous version

The outer import block and the per-class mapping list are nested: one entry inside a many(or(...)) loop that keeps matching either another entry or a newline until it hits the closing }, and each entry itself optionally has its own { mappings } suffix. When both levels use the same {/}, the parser can't reliably tell whether a given } closes the entry's own mapping list or the whole outer block, since chevrotain's LL(k) parser commits to an interpretation using fixed lookahead rather than backtracking. In practice this showed up as the parser splitting one entry into two: the mapping list's own { got misread as the start of a brand new CsvClassImport, and the AST had a second, bogus import whose "class" was actually one of the mapping tokens, failing to resolve as a reference.

Switching the mapping list to [ ] instead removes the ambiguity entirely, since chevrotain can now distinguish which closing bracket belongs to which nesting level by token type instead of guessing from context.

@szschaler on whether this is a main-parsing-algorithm issue: not really, in the sense that this isn't a Langium/chevrotain bug. It's a known, general limitation of LL(k) parsers: reusing the same delimiter for two nested optional/repeated structures makes the grammar locally ambiguous, and the standard fix in any LL(k)-based grammar (not specific to this codebase) is exactly what I did here, giving each nesting level its own distinct delimiter. @nk-coding might want to weigh in if there's a broader pattern here worth documenting for future grammar additions, but I don't think it needs a separate issue against the parser itself.

I really do not have a clue what is going on here
There are more then enough nested curly bracket things in the various DSLs, there should definitely be a way to do this, just have a look how it is done there
I would like to avoid the square brackets for this part of the grammar

You're right, looked at how model-transformation handles this: match { pattern } then { block } uses two adjacent brace blocks disambiguated by the keyword between them ("then"), not by using a different bracket type. Applied the same technique here: the mapping list is now introduced by a "with" keyword instead of square brackets:

Person from "file.csv" with {
"column" = property
}

Verified live: two CSV imports in the same block, the first with an explicit mapping, parse correctly now with no ambiguity errors. Pushed.

Why are we writing grammars in abstract syntax notation rather than as EBNF-y text files? I find the current code utterly unreadable.

@szschaler this predates my work (it's the pattern Niklas set up for the whole codebase, e.g. the metamodel/model/model-transformation/config grammars all use it), so he can give the definitive answer, but here's my understanding of why.

A normal Langium project defines its grammar in a static .langium EBNF file, compiled once at build time. This codebase's plugins need something a .langium file can't do: contribution plugins (like this CSV one) get loaded and merged into a host language's grammar (the Model language) at runtime, and which contributions are present isn't known until then. GrammarSerializer/GrammarDeserializer (in language-common) exist specifically to take a grammar built via createRule().as(...) and turn it into a plain JSON-like structure that can be shipped over the wire from a plugin's service and merged into the host grammar's rule set at runtime, something you can't do with a .langium file since Langium compiles that ahead of time into a fixed parser.

So the abstract syntax builder isn't chosen for its own sake, it's a workaround for needing runtime-composable grammars, which is the whole point of the contribution-plugin architecture. I agree it's harder to read than an EBNF file, that trade-off is inherent to supporting plugins this way rather than something specific to the CSV grammar.

Basically that abstract syntax is an internal TS DSL for what regular Langium projecta do via its external DSL
it's not that much different, both are transpiled to the Langium AST
for the reason why I did this: exactly as described, to allow for this runtime composition
which would have been possible with a huge hack and the regular DSL
at least for me it was not that hard to read and write after an initial adjustment phase, at least compared to the actual Langium AST json structure
but if this will ever really be an issue in the future, it should be easy to add an Langium external DSL parser via its dependency too

Thanks. It's probably worth splitting this out as a separate issue: #30.

I still don't understand why this forces us to have different parentheses, though. The rules for import and column mapping have different first tokens, so are clearly differentiable by an LL parser. Why does this not happen in the composed grammar?

You were right, and my earlier explanation was wrong. I tested it instead of reasoning about it this time.

I removed the with keyword, rebuilt the services, and parsed this against the composed grammar:

using "./test.mm"

import CSV {
    Person from "staff.csv" {
        "full_name" = name
    }
    Company from "employees.csv"
}

It parses with no errors, no ambiguity warnings from Chevrotain, and imports correctly (Person_0/Person_1 from the mapped file, Company_0/Company_1 from the unmapped one). So there is no ambiguity, and nothing forced a different delimiter.

Your reasoning was the correct one: the mapping block can only start with {, while the enclosing import list either continues with an ID or ends with }, so a single token of lookahead is enough to tell them apart.

The comparison I drew to match { pattern } then { block } was also wrong. That rule has two adjacent brace blocks, where the keyword does carry information. Here there is one optional block after a string, which is not the same situation.

I have pushed the change: the mapping now uses plain nested braces, which is also what @nk-coding originally asked for.

Person from "staff.csv" {
    "full_name" = name
}

I should not have justified a syntax change with a parser limitation I had not verified. Sorry for the detour.

Thanks for looking into this again and finding a fix.

maybe to make that thing a bit clearer: the concept "CSV" should not exist in language/editor/protocol/service-model
this is always an architecture smell as the general model language should not know about the CSV support in this plugin archtecture